Disable TWAI CLKOUT when initializing peripheral #1949
Conversation
msvisser
force-pushed
the
twai-set-clock-off
branch
3 times, most recently
from
46423f2 to
c0d662a
Compare
| @@ -791,6 +791,11 @@ where | |||
| .err_warning_limit() | |||
| .write(|w| unsafe { w.err_warning_limit().bits(96) }); | |||
|
|
|||
| // Disable CLKOUT | |||
There was a problem hiding this comment.
Please explain why you added this piece of code, not (or not only, I guess...) what it does. It's completely unobvious to me (as an uninformed reader) why this has any effect.
Also, do we want this done for all MCUs, when the issue only seems to affect C3? Slightly related, do we want to have a test case in our test suite for this? (I think yes!)
There was a problem hiding this comment.
Currently there is no support for using the CLKOUT functionality of the TWAI peripheral in esp-hal, so I do not think there is a reason to leave the clock divider active. This also matches the behaviour from the ESP-IDF, which in the default configuration also does not use the CLKOUT functionality.
In the default configuration clkout_divider is zero, and clkout_gpio is TWAI_IO_UNUSED. The clkout_divider configuration is passed from twai_driver_install_v2 to twai_hal_configure, which in turn calls twai_ll_set_clkout (https://github.com/espressif/esp-idf/blob/master/components/driver/twai/twai.c#L475 and https://github.com/espressif/esp-idf/blob/master/components/hal/twai_hal.c#L64).
The function twai_ll_set_clkout is chip specific, but for all chips with a TWAI peripheral does the exact same thing when passed zero. It sets the TWAI_CLOCK_OFF bit and sets TWAI_CD to zero. See esp32, esp32c3, esp32c6, esp32p4, esp32s2 and esp32s3. So I think it makes sense to match this configuration.
Why exactly this fixes the issue with receiving on GPIO1, I do not know. In theory leaving the clock divider active should not influence the receive operation, but I guess it does. If esp-hal is going to support the CLKOUT functionality, then we should probably add a note about receiving on GPIO1 when CLKOUT is enabled.
As for the test case, could you point me in the right direction where to add the test?
This fixes the issue with receiving on GPIO1 on the ESP32-C3.
msvisser
force-pushed
the
twai-set-clock-off
branch
from
c0d662a to
025eae3
Compare
|
It seems I was mistaken, issue #1207 was already resolved by #1906. Since the peripheral was always initialized as listen only and I was trying to transmit and then receive, it looked like it was still broken. Then when I forked the repo to build a fix I pulled a slightly newer version with #1929, which actually allows you to set normal mode instead of listen only and that made my code work. So there might be no need to merge this pull request, although it does match the ESP-IDF initialization better this way. Let me know how you want to proceed. |
|
I think if #1906 solved your issue, we can close this for now. If it turns out this also needs to be merged for correct operation, we can revisit. Thanks for the PR and investigation! |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
Description
This change set the
TWAI_CLOCK_OFFbit in theTWAI_CLOCK_DIVIDER_REGon initialization of the TWAI peripheral. This fixes the issues with the ESP32-C3 being unable to receive on GPIO1 (#1207). Additionally it setsTWAI_CDto zero to be sure, although it should already be zero after reset.Testing
I have tested sending and receiving messages using GPIO1 as RX and GPIO2 as TX on an ESP32-C3.